init compressDrv and compressDrvWeb#292324
Conversation
motiejus
force-pushed
the
compress-drv
branch
from
1b152a0 to
4cc424b
Compare
motiejus
force-pushed
the
compress-drv
branch
from
4cc424b to
bec58a8
Compare
|
@ofborg build tests.compress-drv |
ofborg
bot
added
8.has: package (new)
motiejus
requested review from
Ma27,
disassembler,
kolaente and
techknowlogick
|
adding @K900 who suggested the original idea cc @disassembler @kolaente @Ma27 @techknowlogick gitea maintainers. |
|
Mind fixing the merge conflicts? I'll take a closer look after that. |
|
Sure, I'll do it within the next couple of days. |
Rebased. |
philiptaron
left a comment
philiptaron
left a comment
There was a problem hiding this comment.
Several non-blocking comments.
|
Thanks @motiejus! My apologies for submitting a PR to the Gitea derivation earlier that conflicted this PR. I'm not familiar enough with this area of change to be able to offer any other feedback than "Looks good to me, and works for Gitea". From my limited perspective this appears to be a good generalization, and being able to utilize it for other derivations is definitely a big positive. |
|
Thank you for your comments. I've addressed all of them in my private branch, but discovered that the test is flaky. Turns out process substitution with I've been trying different approaches without process substitution, but didn't get there yet. Will re-request your review with v2. |
|
Mind marking this as draft while working on that, @motiejus? I'll be happy to review when you're ready. |
github-actions
bot
added
6.topic: nixos
motiejus
force-pushed
the
compress-drv
branch
3 times, most recently
from
3bd8637 to
9f3dec4
Compare
|
There was something off with I opted for a simple, though not 100% efficient approach (may come back to it later). This seems to work with everything. |
philiptaron
left a comment
philiptaron
left a comment
There was a problem hiding this comment.
Everything checks out. It's a nice and simple PR now.
There was a problem hiding this comment.
I love to see lndir gone here.
There was a problem hiding this comment.
Out of curiosity, why is lndir that bad?
There was a problem hiding this comment.
It's not bad per se -- it's just part of the the xorg world, and replacing it with something that's natively in the stdenvNoCC is both simpler and requires less learning.
There was a problem hiding this comment.
I actually like lndir more, because it's quite widely used, a single operation (instead of two traversals) and automatically resolves nested symlinks (which the previous PR version didn't). I just couldn't get it to work before, and no longer understand/remember why.
There was a problem hiding this comment.
I am not aware of any tool replacing lndir
philiptaron
added
the
12.approvals: 1
There was a problem hiding this comment.
Out of curiosity, why is lndir that bad?
There was a problem hiding this comment.
My knowledge on the documentation infrastructure is a little outdated, so excuse my ignorance, but does this doc-comment appear in one of the manuals?
There was a problem hiding this comment.
I am not sure how to check; I mostly cargo-culted the comment format from the functions I like in lib/.
There was a problem hiding this comment.
cc @NixOS/documentation-team any suggestions?
There was a problem hiding this comment.
Using doc-comments would be beneficial, since they can be retrieved with :doc in the repl. since release 2.24
And we plan to render them into the manual.
/** */
Content should be markdown, so its rendered properly
My knowledge on the documentation infrastructure is a little outdated, so excuse my ignorance, but does this doc-comment appear in one of the manuals?
No not yet.
@motiejus It is planned to migrate all doc comments into one common format:
(:doc doesn't work in this particular case, because some features are still missing, but https://noogle.dev should find it after its merged)
There was a problem hiding this comment.
I did my best following the examples here and in attrsets.
My system is still on 2.18, which makes it nontrivial to see the end result. I'll have a look once this lands somewhere or my nix is 2.24.
motiejus
force-pushed
the
compress-drv
branch
3 times, most recently
from
1da5bbe to
1e6c24b
Compare
*compressDrv* compresses files in a given derivation. *compressDrvWeb* compresses a derivation for a loosely-defined pre-compressed "web server" usage. This intends to replace the `passthru.data-compressed` derivations that have accumulated in nixpkgs with something more reusable.
|
Looks like it could use a little markdown editing to look a bit better, but it's working! |
I've done that If you want to merge it |
This is a leftover from the original versions of NixOS#292324. Reduces the API surface to not accept accidental arguments.
|
@Ma27 to be able to use this for nextcloud we also need to pass |
|
I am currently doing a PR with some small improvements including zstd support which chrome is already using. PS Coincidentally this broke eval because I used that name locally already 😂 Edit: #332752 |
6 participants
Description of changes
add compressDrv and compressDrvWeb. I have been working on adding gamja with
passthru.data-compressedand @K900 suggested to add something generic. This is my first attempt.Things done
compressDrv compresses files in a given derivation.
compressDrvWeb compresses a derivation for a loosely-defined
pre-compressed "web server" usage.
This intends to replace the
passthru.data-compressedderivations that have accumulated in nixpkgs with something more reusable. First one is gitea.nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)